Skip to content
This repository has been archived by the owner on Nov 6, 2019. It is now read-only.

Add support for filtering change types when diffing #303

Merged
merged 2 commits into from
Nov 29, 2017

Conversation

lukebjerring
Copy link
Collaborator

@lukebjerring lukebjerring commented Nov 27, 2017

Progresses #301

Uses a git-inspired filtering charset param.

?filter=[A][C][D]
A = Added
C = Changed
D = Deleted

diff[test] = []int{
max(passDiff, countDiff),
max(resultsBefore[1], resultsAfter[1]),
if filter.Deleted || filter.Changed {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This diff tool isn't great. FWIW, changes are just:

  • Surround whole code-block in the if-statement above
  • Add continue if-statements

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, very helpful.

param = DiffFilterParam{}
for _, char := range filter {
switch char {
case 'A':
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment around here as well that this is inspired by git --diff-filter=, which, BTW, I really like :) I don't think that rename detection is in any way a straightforward thing, or likely to ever be worth doing, but if we do, it should be 'R'.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noted, and Done.

diff[test] = []int{
max(passDiff, countDiff),
max(resultsBefore[1], resultsAfter[1]),
if filter.Deleted || filter.Changed {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, very helpful.

@lukebjerring lukebjerring merged commit 09c5ddd into web-platform-tests:master Nov 29, 2017
@lukebjerring lukebjerring deleted the diff-filter branch November 29, 2017 17:26
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants